Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add lz4 to SnapshotSegment::Headers #6487

Merged
merged 4 commits into from
Feb 14, 2024

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Feb 8, 2024

Adds compression to Header static file segments.

Holesky near tip
Before: 510MB
After: 352MB

On previous tests done before, there wasn't a noticeable impact on read performance, but will use flood to confirm it. So pending merge

Not adding for other segments, since they already leverage the codec compression (global zstd dictionary)

@joshieDo joshieDo added the A-static-files Related to static files label Feb 8, 2024
@joshieDo joshieDo requested a review from rakita as a code owner February 8, 2024 12:55
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥
lgtm, pending @joshieDo @shekhirin


// Transaction and Receipt already have the compression scheme used natively in its encoding.
// (zstd-dictionary)
if matches!(segment, SnapshotSegment::Headers) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can add fn is_ for segment separately

Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice. Love that it's a small change LOC-wise.

@shekhirin shekhirin force-pushed the feat/static-files branch 5 times, most recently from dd6aa63 to 613f319 Compare February 8, 2024 17:03
@joshieDo
Copy link
Collaborator Author

joshieDo commented Feb 14, 2024

cryo blocks --blocks 0:900k --csv

Run this a couple times on both branches (with a cherry picked from #6608) and didn't see a difference so merging this.

@joshieDo joshieDo merged commit 1eb9181 into feat/static-files Feb 14, 2024
22 of 24 checks passed
@joshieDo joshieDo deleted the feat/header-compress branch February 14, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-static-files Related to static files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants